Skip to content

translate/upsert: use emit_column_or_rowid for DO UPDATE row snapshot#6118

Merged
PThorpe92 merged 4 commits intotursodatabase:mainfrom
ongyimeng:fix/upsert-added-column-default-compat
Mar 25, 2026
Merged

translate/upsert: use emit_column_or_rowid for DO UPDATE row snapshot#6118
PThorpe92 merged 4 commits intotursodatabase:mainfrom
ongyimeng:fix/upsert-added-column-default-compat

Conversation

@ongyimeng
Copy link
Contributor

Description

  • In core/translate/upsert.rs, conflicting-row snapshots now use emit_column_or_rowid(...)
  • Replaces manual Column { default: None } handling
  • Adds regression test in testing/sqltests/tests/upsert.sqltest

Motivation and context

  • UPSERT was reading NULL in cases where SQLite expects the column’s default value
  • This happens for missing trailing column values after ALTER TABLE ... ADD COLUMN ... DEFAULT ...
  • The fix restores SQLite-compatible behavior
  • Applies specifically to the DO UPDATE path of UPSERT

Issue #6087

Description of AI Usage

Used AI to trace the issue to upsert.rs, propose the minimal fix, and draft the regression test.

Copy link

@turso-bot turso-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please review @LeMikaelF

@ongyimeng ongyimeng marked this pull request as draft March 24, 2026 09:55
@ongyimeng ongyimeng marked this pull request as ready for review March 24, 2026 14:17
Copy link

@turso-bot turso-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please review @LeMikaelF

@ongyimeng
Copy link
Contributor Author

could someone please help rerun the failed CI jobs for this PR? I believe the failures are infra/flaky

@PThorpe92 PThorpe92 force-pushed the fix/upsert-added-column-default-compat branch from b2a4ae2 to 1b65689 Compare March 25, 2026 21:27
@LeMikaelF
Copy link
Collaborator

@ongyimeng you can ignore the sync and FK test failures. We know about them and they're being worked on. One of the snapshot tests was failing on main yesterday, but it was fixed a few hours ago, so rebasing on main should help.

@PThorpe92
Copy link
Collaborator

hey @ongyimeng I fixed the conflicts and pushed, should be good to go as soon as CI passes.

Thanks for the contribution!

@PThorpe92 PThorpe92 merged commit 9b8634b into tursodatabase:main Mar 25, 2026
91 of 92 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants